benchmark: add test runner hooks and options#63754
Conversation
Add benchmarks for node:test hooks and test options. The hooks benchmark covers before, after, beforeEach, and afterEach, with a none mode as the baseline. The test options benchmark covers skip and todo behavior. This adds coverage for part of the benchmark/test_runner gaps tracked in the issue. Refs: nodejs#55723 Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>
|
Review requested:
|
nodejs-github-bot
added
benchmark
| describe(`${i}`, () => { | ||
| switch (hook) { | ||
| case 'before': | ||
| before(noop); | ||
| break; | ||
| case 'after': | ||
| after(noop); | ||
| break; | ||
| case 'beforeEach': | ||
| beforeEach(noop); | ||
| break; | ||
| case 'afterEach': | ||
| afterEach(noop); | ||
| break; | ||
| case 'none': | ||
| break; | ||
| } |
There was a problem hiding this comment.
This is also benchmarking the switch statement, which we don't want.
Select each benchmark variant before starting the timer so the measured loop does not include dispatch overhead. Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>
|
@avivkeller Changes done! Obs: |
| // eslint-disable-next-line prefer-const | ||
| let avoidV8Optimization = 0; |
There was a problem hiding this comment.
It is let because we rewrite the values through the benchmark, and there is no particular reason to be 0, just didn't want to leave it undefined, and the original value of the variable shouldn't have an impact on this.
To be 100% honest, I don't quite understand the mechanics for this. I never dug into v8 optimizations before. The variable is there because I decided to follow the existing patterns inside this particular folder.
But please check this reference link and this for the comment that originally introduced this specific variable (Seems like the link is not working 100%, opening the discussion so just crtl+f for benchmark/test_runner/global-concurrent-tests.js and there should be a resolved review from H4ad with the reference)
Obs: I just realized the hooks are noops, and given that this variable works as described, I shouldn't have a noop there, so I'm pushing code so the hooks also have minimal execution within the function. Thanks for calling my attention to this.
Update the hooks benchmark so registered hooks perform the same anti-optimization assignment as test bodies instead of calling a noop. This keeps the measured hook path from using an empty callback. Links for more information on this actions: nodejs#48931 (comment) https://www.mail-archive.com/v8-users@googlegroups.com/msg05521.html Signed-off-by: Luan Muniz <luan@luanmuniz.com.br>
4 participants
Add benchmarks for node:test hooks and test options. The hooks benchmark covers before, after, beforeEach, and afterEach, with a none mode as the baseline. The test options benchmark covers skip and todo behavior. This adds coverage for part of the benchmark/test_runner gaps tracked in the issue.
Refs: #55723
I eventually want to continue working on the Ref issue, and do the other benchmarks, but I decided to start with the simple ones that I can use existing files as a reference, get a feel for how the project works. Starting small and all of that. This will be my first (hopefully of many) contribution.
Please let me know if any changes or actions are required from me. I will wait for feedback before start tackling the other parts of the ref issue
Edit: I had some time today, and I started working on the other benchmarks, and I do have
only,mock.timersandmock.modulebenchmarks ready too. I will await for feedback on the current code before pushing the other benchmarks. Please let me know if you prefer for me to push a different PR those since they have a bit more nuance, especially the mock.module one (Still nothing major tho)